BUG: hidden app roots (v2) #107

ryan-roemer · 2019-03-20T22:07:28Z

Version 2. Supersedes #104

When an application root has no actual node_modules in it, the versions action inference fails to find the app root package.json file and misses traversing the versions dependencies. This has the buggy effects of:

Missing information in versions action for CLI
Missing information in DuplicatesPlugin.

Work

Create regression test fixtures
Create regression tests
Create fix. Fixes [BUG] Application roots without node_modules on disk are missed for versions inference. #103
Address all TODOs and clean up.

codecov · 2019-03-21T21:14:31Z

Codecov Report

Merging #107 into master will increase coverage by 0.18%.
The diff coverage is 96.03%.

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
+ Coverage   90.77%   90.95%   +0.18%     
==========================================
  Files          14       15       +1     
  Lines         694      730      +36     
  Branches      111      116       +5     
==========================================
+ Hits          630      664      +34     
- Misses         38       39       +1     
- Partials       26       27       +1

Impacted Files	Coverage Δ
src/lib/util/promise.ts	`100% <100%> (ø)`
src/lib/actions/base.ts	`93.2% <100%> (+0.06%)`	⬆️
src/lib/util/dependencies.ts	`95.13% <92%> (+0.21%)`	⬆️
src/lib/actions/versions.ts	`94.76% <97.22%> (-0.44%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe5e41f...54c49a5. Read the comment docs.

codecov · 2019-03-21T21:14:31Z

Codecov Report

Merging #107 into master will decrease coverage by 0.21%.
The diff coverage is 92.7%.

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
- Coverage   90.77%   90.56%   -0.22%     
==========================================
  Files          14       15       +1     
  Lines         694      763      +69     
  Branches      111      122      +11     
==========================================
+ Hits          630      691      +61     
- Misses         38       41       +3     
- Partials       26       31       +5

Impacted Files	Coverage Δ
src/lib/util/promise.ts	`100% <100%> (ø)`
src/lib/actions/base.ts	`93.2% <100%> (+0.06%)`	⬆️
src/plugin/duplicates.ts	`86.3% <80%> (-1.11%)`	⬇️
src/lib/util/dependencies.ts	`95.1% <91.66%> (+0.17%)`	⬆️
src/lib/actions/versions.ts	`94.11% <95.5%> (-1.09%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe5e41f...18bd729. Read the comment docs.

…cateplugin.

ryan-roemer · 2019-03-26T03:36:51Z

test/lib/plugin/duplicates.spec.ts

      num: 0,
    },
-    files: {


This fixture has been wrong for a long time...

ryan-roemer · 2019-03-26T16:31:48Z

Released in inspectpack@4.2.0

parkerziegler · 2019-03-26T17:02:37Z

@ryan-roemer I'm going to upgrade webpack-dashboard to use inspectpack@4.2.0 to pick up this change in 3.0.1. Do you think this fixes https://github.com/FormidableLabs/webpack-dashboard/issues/276?

ryan-roemer · 2019-03-26T17:07:26Z

@parkerziegler -- Upgrade sounds great! Unfortunately, it won't fix https://github.com/FormidableLabs/webpack-dashboard/issues/276 which I'm likely going to migrate over to inspectpack as an issue there....

parkerziegler · 2019-03-26T17:22:41Z

Cool, no worries. Thanks a lot for taking that one on, and for this PR!

ryan-roemer added 30 commits February 26, 2019 10:52

Add regression test fixture. Still need regression test.

1d01023

Add regression tests

12a6b6b

Remove unneeded alias

4146d0b

Remove unneeded resolve too

b702591

Fix lint

b31f0ea

Fix fixture pkg.main

85189a4

WIP: Start enhancing regression test and debugging.

fe90927

WIP: Fix fixture stuff and get new error for 'yarn test' to run next

4312602

Get tests back to passing state with skips, etc.

1d42e9b

Add more regression test skeletons

b948902

WIP: Getting closer to cache-based solution

ec38037

Cleanup up and comment out WIP bugfix work for clean push.

6268b19

Experiment: Remove rootPath parameter

af184c8

Add unit test for readPackages

a4601a5

Start adding _findPackage tests and remove resolves

23c748c

Add serial promise utility.

db2d2ec

Switch to serial traversal

cf71cb8

More comments

c8b6357

more envvar

564c598

Get tests up and running with manually hacked in extra project root

0ae24a2

Various cleanup

7d8eb96

Start adding tests for _packageRoots

be36368

Add failing test

0a61c7c

Add more notes

6f6f879

more notes

a251bda

WIP/experiment with tempPotentialAppRoots

2dc042a

WIP: More notes

c4dd88a

Clean up _packageRoots

f554117

Refactor _packageRoots to be async

57a37fe

Introspect potential roots for package.json

5b926ef

Only add unique modules to versions output

300b1a2

ryan-roemer mentioned this pull request Mar 20, 2019

[WIP] BUG: hidden app roots #104

Closed

3 tasks

ryan-roemer marked this pull request as ready for review March 20, 2019 22:08

ryan-roemer closed this Mar 20, 2019

ryan-roemer reopened this Mar 20, 2019

ryan-roemer added 5 commits March 21, 2019 12:55

Start adding tests and cleaning up TODOs

64fa435

Clean up TODOs

7415d52

More TODO cleanup

dd6763a

More cleanup

592a36c

more cleanup

54c49a5

ryan-roemer added 8 commits March 21, 2019 15:46

Move webpack token removal

25a6f8a

WIP: Broken. More common path work.

32caab0

Switch package roots to common root.

3e3a381

Add commonRoot meta

275afad

Implement test

e102e43

Clean up todos

373be80

Fix test

b3567ef

Add warning guardrails for expect vs actual matched sources for dupli…

1741c97

…cateplugin.

ryan-roemer changed the title ~~[WIP] BUG: hidden app roots (v2)~~ BUG: hidden app roots (v2) Mar 25, 2019

Make tests windows friendly

18bd729

ryan-roemer mentioned this pull request Mar 26, 2019

[BUG] Application roots without node_modules on disk are missed for versions inference. #103

Closed

ryan-roemer commented Mar 26, 2019

View reviewed changes

test/lib/plugin/duplicates.spec.ts

num: 0,

},

files: {

Copy link

Member Author

ryan-roemer Mar 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixture has been wrong for a long time...

ryan-roemer merged commit 0c2e588 into master Mar 26, 2019

ryan-roemer deleted the bug/hidden-app-roots-v2 branch March 26, 2019 16:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: hidden app roots (v2) #107

BUG: hidden app roots (v2) #107

ryan-roemer commented Mar 20, 2019 •

edited

Loading

codecov bot commented Mar 21, 2019

codecov bot commented Mar 21, 2019 •

edited

Loading

ryan-roemer Mar 26, 2019

ryan-roemer commented Mar 26, 2019

parkerziegler commented Mar 26, 2019

ryan-roemer commented Mar 26, 2019

parkerziegler commented Mar 26, 2019

BUG: hidden app roots (v2) #107

BUG: hidden app roots (v2) #107

Conversation

ryan-roemer commented Mar 20, 2019 • edited Loading

Work

codecov bot commented Mar 21, 2019

Codecov Report

codecov bot commented Mar 21, 2019 • edited Loading

Codecov Report

ryan-roemer Mar 26, 2019

Choose a reason for hiding this comment

ryan-roemer commented Mar 26, 2019

parkerziegler commented Mar 26, 2019

ryan-roemer commented Mar 26, 2019

parkerziegler commented Mar 26, 2019

ryan-roemer commented Mar 20, 2019 •

edited

Loading

codecov bot commented Mar 21, 2019 •

edited

Loading